Is there a concern that this won't stop?

@S1nus @tzdybal not sure why we need a separate package "full"? we could simply put them all under "node" package:

node
     - client.go
     - full.go
     - node.go (interface)
     - light.go
     - lightclient.go
     ...

will this not be sufficient? i would only make separate package if we expect repeated functionalities for full and light nodes. But then, we would end up duplicating lot of code.
in general, what services (functionalities) do we anticipate for light node?

I requested for introducing directories, because I felt like full_node.go and full_client.go + all tests files, will make a bit of a mess sooner than later.

For some context: #661

Both full and light nodes will implement simplified Node interface - which basically allows us to Start a node and create rpc.Client compatible with cosmos-sdk.
Client implementation returned by full node will be completely different than the one returned by light node. I don't expect to share a lot of code between light and full nodes & clients.

I requested for introducing directories, because I felt like full_node.go and full_client.go + all tests files, will make a bit of a mess sooner than later.

For some context: #661

Both full and light nodes will implement simplified Node interface - which basically allows us to Start a node and create rpc.Client compatible with cosmos-sdk. Client implementation returned by full node will be completely different than the one returned by light node. I don't expect to share a lot of code between light and full nodes & clients.

in this case, why not have this?

node
     - client.go
     - full.go
     - node.go (interface)
     - light.go
     - lightclient.go
     ...

this clearly distinguish full and light implementations and then client refers to full client and for light node client, we can use lightclient?

Codecov Report

Merging #681 (3379369) into main (d8679cc) will increase coverage by 0.08%.
The diff coverage is 53.31%.

@@            Coverage Diff             @@
##             main     #681      +/-   ##
==========================================
+ Coverage   54.49%   54.57%   +0.08%     
==========================================
  Files          49       52       +3     
  Lines       10305    10372      +67     
==========================================
+ Hits         5616     5661      +45     
- Misses       3820     3842      +22     
  Partials      869      869              

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

I requested for introducing directories, because I felt like full_node.go and full_client.go + all tests files, will make a bit of a mess sooner than later.
For some context: #661
Both full and light nodes will implement simplified Node interface - which basically allows us to Start a node and create rpc.Client compatible with cosmos-sdk. Client implementation returned by full node will be completely different than the one returned by light node. I don't expect to share a lot of code between light and full nodes & clients.

in this case, why not have this?

node
     - client.go
     - full.go
     - node.go (interface)
     - light.go
     - lightclient.go
     ...

this clearly distinguish full and light implementations and then client refers to full client and for light node client, we can use lightclient?

one nice advantage of this organization is that i can make NewFullNode private and accessed through NewNode via the config option.

but i don't have a strong preference, it works either way

You convinced me! Let's use convention proposed by @gupadhyaya .

@S1nus you should also enforce the interface binding by adding var _ Node = &FullNode{} to full.go and var _ Node = &LightNode{} to light.go. similar to how it is done in full_client.go var _ rpcclient.Client = &FullClient{}, also do it for light_client.go to enforce rpcclient.Client interface binding.

I think we should close channel when exiting eventsRoutine in FullClient (using defer).

@tzdybal
what's the purpose of the "resubscribe" part? Should the channel (outc?) be closed on c.Quit or something else?

what's the purpose of the "resubscribe" part?

Tendermint compatibility.

Should the channel (outc?) be closed on c.Quit or something else?

As function has multiple return statemets, it's best to use defer statement on top of function body, like:

func (c *Client) eventsRoutine(sub types.Subscription, subscriber string, q tmpubsub.Query, outc chan<- ctypes.ResultEvent) {
	defer close(outc)
// ...
}

As function has multiple return statemets, it's best to use defer statement on top of function body, like:

fixed?

@tzdybal @gupadhyaya Going back to the discussion of folder structure/abstraction, as I was not part of that conversation yet. Did we consider the option of having the same architecture as celestia-node? https://github.com/celestiaorg/celestia-node/tree/main/nodebuilder

Did we consider the option of having the same architecture as celestia-node?

Hmm not sure. It is similar, the main difference is RollKit does not use Fx.